Skip to content

CLI: Remove random commas in storybook upgrade logs#22333

Merged
JReinhold merged 4 commits into
storybookjs:nextfrom
joaonunomota:no-more-commas
Aug 29, 2023
Merged

CLI: Remove random commas in storybook upgrade logs#22333
JReinhold merged 4 commits into
storybookjs:nextfrom
joaonunomota:no-more-commas

Conversation

@joaonunomota
Copy link
Copy Markdown
Contributor

@joaonunomota joaonunomota commented May 1, 2023

Closes #22298

What I did

Log the stdout and stderr properties, instead of logging output property of result returned by npm-check-updates@latest.

This provides the following benefits:

  • Avoids commas that come from logging an array, which solves [Bug]: some extra comma #22298.
  • It is clearer in the code what is being logged, since stdout and stderr are equal to output[1] and output[2] respectively.
  • stderr can be logged at a different level to stdout (not sure what the rule for using info or warn on logs)

It also:

  • Does not log output[0] which in this case is null.
  • Adds an extra newline between stdout and stderr logs

An example of what the logs look like before and after with some forced warnings (using reproduction code):

image

How to test

  1. Run a sandbox for template, e.g. yarn task --task sandbox --start-from auto --template react-vite/default-ts
  2. Run npx storybook@latest upgrade
  3. Observe logs, specifically for npm-check-updates

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

Prevents commas from being logged on the terminal
Adds newline between stdout and stderr
@joaonunomota joaonunomota changed the title Log stdout and stderr from npm-check-updates separately Fix: Remove random commas in storybook upgrade logs May 1, 2023
Copy link
Copy Markdown
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! 💪

@JReinhold JReinhold changed the title Fix: Remove random commas in storybook upgrade logs CLI: Remove random commas in storybook upgrade logs Aug 29, 2023
@JReinhold JReinhold added the ci:normal Run our default set of CI jobs (choose this for most PRs). label Aug 29, 2023
@JReinhold JReinhold merged commit d23d577 into storybookjs:next Aug 29, 2023
@joaonunomota joaonunomota deleted the no-more-commas branch August 29, 2023 11:21
@yannbf yannbf mentioned this pull request Oct 17, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:normal Run our default set of CI jobs (choose this for most PRs). cli

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: some extra comma

2 participants